Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

eks_nodegroup - fixing remote access and added to integration tests #1773

Merged

Conversation

romulus-ai
Copy link
Contributor

SUMMARY

Fixes #1771

Handling remote_access configuration the right way that boto understands it. Also included it to integration tests.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

eks_nodegroup

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.
https://ansible.softwarefactory-project.io/zuul/buildset/65ab7e1be5a7482e853cec5d8668c029

✔️ ansible-galaxy-importer SUCCESS in 3m 41s
✔️ build-ansible-collection SUCCESS in 12m 50s
ansible-test-sanity-docker-devel FAILURE in 8m 51s (non-voting)
ansible-test-sanity-docker-milestone FAILURE in 8m 46s (non-voting)
ansible-test-sanity-docker-stable-2.12 FAILURE in 9m 06s
ansible-test-sanity-docker-stable-2.13 FAILURE in 10m 50s
ansible-test-sanity-docker-stable-2.14 FAILURE in 8m 43s
✔️ ansible-test-units-amazon-aws-python36 SUCCESS in 7m 51s
✔️ ansible-test-units-amazon-aws-python38 SUCCESS in 6m 20s
✔️ ansible-test-units-amazon-aws-python39 SUCCESS in 6m 12s
✔️ ansible-test-units-amazon-aws-python310 SUCCESS in 7m 26s
ansible-test-changelog FAILURE in 4m 26s
✔️ ansible-test-splitter SUCCESS in 4m 59s
✔️ integration-community.aws-1 SUCCESS in 24m 01s
Skipped 21 jobs

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/367c90817831459da120a8fef830e28c

✔️ ansible-galaxy-importer SUCCESS in 5m 16s
✔️ build-ansible-collection SUCCESS in 12m 36s
ansible-test-sanity-docker-devel FAILURE in 13m 28s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 12m 04s (non-voting)
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 12m 19s
✔️ ansible-test-sanity-docker-stable-2.13 SUCCESS in 12m 13s
✔️ ansible-test-sanity-docker-stable-2.14 SUCCESS in 12m 45s
✔️ ansible-test-units-amazon-aws-python36 SUCCESS in 8m 25s
✔️ ansible-test-units-amazon-aws-python38 SUCCESS in 8m 51s
✔️ ansible-test-units-amazon-aws-python39 SUCCESS in 7m 35s
✔️ ansible-test-units-amazon-aws-python310 SUCCESS in 9m 22s
✔️ ansible-test-changelog SUCCESS in 4m 30s
✔️ ansible-test-splitter SUCCESS in 4m 50s
✔️ integration-community.aws-1 SUCCESS in 29m 11s
Skipped 21 jobs

@romulus-ai
Copy link
Contributor Author

Hey 👋 can someone takea look at this? Its a quiet small fix :)

@tremble tremble changed the title fixing remote access and added to integration tests eks_nodegroup - fixing remote access and added to integration tests Apr 17, 2023
Comment on lines +109 to +125

- name: Create securitygroup for node access
amazon.aws.ec2_security_group:
name: 'ansible-test-eks_nodegroup'
description: "SSH access"
vpc_id: '{{ setup_vpc.vpc.id }}'
rules:
- proto: tcp
ports:
- 22
cidr_ip: 0.0.0.0/0
register: securitygroup_eks_nodegroup

- name: Create Keypair for Access to Nodegroup nodes
amazon.aws.ec2_key:
name: "ansible-test-eks_nodegroup"
register: ec2_key_eks_nodegroup
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@romulus-ai can you also add both ressources to the cleanup.yml to take care, they are removed after testing is finished?
otherwise looks good to me!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@markuman markuman added the backport-5 PR should be backported to the stable-5 branch label Apr 17, 2023
@softwarefactory-project-zuul
Copy link
Contributor

Build failed.
https://ansible.softwarefactory-project.io/zuul/buildset/93ca2404a2e34cb2afc675bba459a5a6

ansible-galaxy-importer FAILURE in 4m 01s
✔️ build-ansible-collection SUCCESS in 13m 02s
ansible-test-sanity-docker-devel FAILURE in 15m 35s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 8m 47s (non-voting)
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 9m 05s
✔️ ansible-test-sanity-docker-stable-2.13 SUCCESS in 10m 05s
✔️ ansible-test-sanity-docker-stable-2.14 SUCCESS in 8m 46s
✔️ ansible-test-units-amazon-aws-python36 SUCCESS in 5m 36s
✔️ ansible-test-units-amazon-aws-python38 SUCCESS in 5m 55s
✔️ ansible-test-units-amazon-aws-python39 SUCCESS in 7m 33s
✔️ ansible-test-units-amazon-aws-python310 SUCCESS in 6m 37s
✔️ ansible-test-changelog SUCCESS in 4m 19s
✔️ ansible-test-splitter SUCCESS in 4m 45s
✔️ integration-community.aws-1 SUCCESS in 24m 28s
Skipped 21 jobs

@markuman markuman requested a review from tremble April 19, 2023 16:04
@tremble tremble added the mergeit Merge the PR (SoftwareFactory) label Apr 19, 2023
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded (gate pipeline).
https://ansible.softwarefactory-project.io/zuul/buildset/a479665455f64800929047fe5ae3c95d

✔️ ansible-galaxy-importer SUCCESS in 3m 46s
✔️ build-ansible-collection SUCCESS in 12m 57s
ansible-test-sanity-docker-devel FAILURE in 10m 03s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 11m 45s (non-voting)
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 10m 37s
✔️ ansible-test-sanity-docker-stable-2.13 SUCCESS in 10m 37s
✔️ ansible-test-sanity-docker-stable-2.14 SUCCESS in 9m 00s
✔️ ansible-test-units-amazon-aws-python36 SUCCESS in 5m 40s
✔️ ansible-test-units-amazon-aws-python38 SUCCESS in 6m 24s
✔️ ansible-test-units-amazon-aws-python39 SUCCESS in 6m 02s
✔️ ansible-test-units-amazon-aws-python310 SUCCESS in 5m 46s
✔️ ansible-test-changelog SUCCESS in 4m 46s
✔️ ansible-test-splitter SUCCESS in 4m 53s
✔️ integration-community.aws-1 SUCCESS in 27m 11s
Skipped 21 jobs

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit ef84541 into ansible-collections:stable-5 Apr 19, 2023
@patchback
Copy link

patchback bot commented Apr 19, 2023

Backport to stable-5: 💔 cherry-picking failed — conflicts found

❌ Failed to cleanly apply ef84541 on top of patchback/backports/stable-5/ef84541446c0df46cc479cc5b6429335c965c360/pr-1773

Backporting merged PR #1773 into stable-5

  1. Ensure you have a local repo clone of your fork. Unless you cloned it
    from the upstream, this would be your origin remote.
  2. Make sure you have an upstream repo added as a remote too. In these
    instructions you'll refer to it by the name upstream. If you don't
    have it, here's how you can add it:
    $ git remote add upstream https://github.com/ansible-collections/community.aws.git
  3. Ensure you have the latest copy of upstream and prepare a branch
    that will hold the backported code:
    $ git fetch upstream
    $ git checkout -b patchback/backports/stable-5/ef84541446c0df46cc479cc5b6429335c965c360/pr-1773 upstream/stable-5
  4. Now, cherry-pick PR eks_nodegroup - fixing remote access and added to integration tests #1773 contents into that branch:
    $ git cherry-pick -x ef84541446c0df46cc479cc5b6429335c965c360
    If it'll yell at you with something like fatal: Commit ef84541446c0df46cc479cc5b6429335c965c360 is a merge but no -m option was given., add -m 1 as follows instead:
    $ git cherry-pick -m1 -x ef84541446c0df46cc479cc5b6429335c965c360
  5. At this point, you'll probably encounter some merge conflicts. You must
    resolve them in to preserve the patch from PR eks_nodegroup - fixing remote access and added to integration tests #1773 as close to the
    original as possible.
  6. Push this branch to your fork on GitHub:
    $ git push origin patchback/backports/stable-5/ef84541446c0df46cc479cc5b6429335c965c360/pr-1773
  7. Create a PR, ensure that the CI is green. If it's not — update it so that
    the tests and any other checks pass. This is it!
    Now relax and wait for the maintainers to process your pull request
    when they have some cycles to do reviews. Don't worry — they'll tell you if
    any improvements are necessary when the time comes!

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

@github-actions
Copy link

Docs Build 📝

Thank you for contribution!✨

This PR has been merged and your docs changes will be incorporated when they are next published.

@tremble tremble removed the backport-5 PR should be backported to the stable-5 branch label Apr 20, 2023
tremble pushed a commit to tremble/community.aws that referenced this pull request Apr 20, 2023
…nsible-collections#1773)

eks_nodegroup - fixing remote access and added to integration tests

SUMMARY
Fixes ansible-collections#1771
Handling remote_access configuration the right way that boto understands it. Also included it to integration tests.
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
eks_nodegroup

Reviewed-by: Markus Bergholz <[email protected]>
Reviewed-by: Thomas Bruckmann
Reviewed-by: Mark Chappell
@tremble
Copy link
Contributor

tremble commented Apr 20, 2023

@romulus-ai

I just noticed that you opened this directly against the stable-5 branch. In general our development is done against the main branch, and backported to the relevant stable-X branches. By opening a PR against the stable branches, there's a risk that the fix will be missing once we release the next major version.

Please only open PRs against the main branch. The only exceptions should be if you're backporting a PR from main into one of the stable branches, or there's a strong reason why the code shouldn't be applied to future major releases (major rewrites, deprecated features, etc).

softwarefactory-project-zuul bot pushed a commit that referenced this pull request Apr 20, 2023
…1773) (#1781)

eks_nodegroup - fixing remote access and added to integration tests

SUMMARY
This was incorrectly merged directly into stable-5 rather than main.
Fixes #1771
Handling remote_access configuration the right way that boto understands it. Also included it to integration tests.
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
eks_nodegroup
ADDITIONAL INFORMATION
This is pulling #1773 from stable-5 into main
Reviewed-by: Markus Bergholz
Reviewed-by: Thomas Bruckmann
Reviewed-by: Mark Chappell

Reviewed-by: Markus Bergholz <[email protected]>
@romulus-ai
Copy link
Contributor Author

Oh, did not know that, can I help somehow to clean that things up?

@tremble
Copy link
Contributor

tremble commented Apr 21, 2023

@romulus-ai We've cleaned up already.

And I forgot to say this last time:

Many thanks for taking the time to contribute to community.aws. This fix should be available once we release 5.5.0 and/or 6.0.0. We hope to have these out in the next couple of weeks.

@romulus-ai
Copy link
Contributor Author

Ok, thanks and sorry again causing some trouble here! Next time I know it :)

abikouo pushed a commit to abikouo/community.aws that referenced this pull request Oct 24, 2023
…e_iam_access_key

Promote iam_access_key and the corresponding _info module
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mergeit Merge the PR (SoftwareFactory)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants